Skip to content

Improve @template lookup and resilience #42851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 18, 2021
Merged

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Feb 18, 2021

  1. @template parsing may produce a template tag with a type parameter whose name is the missing identifier. These tags should be skipped in the checker because they receive an error in the parser.
  2. The fix in Look for outer type parameters on VariableStatements #37819 was incorrect; there's no such thing as a type parameter declared on a variable declaration. Instead, there needs to be a type
    parameter declared on a jsdoc comment, because that's the scope for tags like @return and @typedef.

There are 3 tests because either fix (1) and (2) fix the first test's failure, but both are required to fix the last two tests' failures.

Fixes #42812

Edit: The fix in (1) isn't as good as the one in #42017, so I'm going to take that and leave the test cases in place.

1. @template parsing may produce a template tag with a type parameter
whose name is the missing identifier. These tags should be skipped
in the checker because they receive an error in the parser.
2. The fix in #37819 was incorrect; there's no such thing as a type
parameter declared on a variable declaration. Instead, there needs to be a type
parameter declared on a jsdoc comment, because that's the scope for tags
like `@return` and `@typedef`.

There are 3 tests because either fix (1) and (2) fix the first test's
failure, but both are required to fix the last two tests' failures.
@@ -4408,7 +4408,7 @@ namespace ts {
}

export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters): readonly TypeParameterDeclaration[] {
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters : undefined);
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters.filter(p => !containsParseError(p)) : undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I heard u like Haskell

Suggested change
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters.filter(p => !containsParseError(p)) : undefined);
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters.filter(not(containsParseError)) : undefined);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um....oops. This isn't needed since I found that the Pursuit fellows made a better fix in December.

@sandersn sandersn merged commit 3d7ec8a into master Feb 18, 2021
@sandersn sandersn deleted the improve-template-tag-lookup branch February 18, 2021 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSDoc @return, @template tags crash tsc even with --checkJs=false
3 participants